fix(ui): move Comments/CommentForm to emdash/ui/comments subpath (#2039)#2044
fix(ui): move Comments/CommentForm to emdash/ui/comments subpath (#2039)#2044scottbuscemi wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 201e4b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Scope checkThis PR touches 25 files. PRs with a broad scope are harder to review. Please confirm the scope hasn't drifted beyond the intended change. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 201e4b2 | Jul 17 2026, 06:58 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 201e4b2 | Jul 17 2026, 06:59 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 201e4b2 | Jul 17 2026, 06:56 PM |
There was a problem hiding this comment.
The approach is correct: moving Comments and CommentForm out of the emdash/ui barrel into emdash/ui/comments is the right fix for the Astro CSS-scanning issue described in #2039, and it mirrors the existing emdash/ui/search entry point. The export map, barrel cleanup, tsconfig exclusion, changeset, and in-repo fixture/demo/template .astro updates are all consistent.
I checked the diff, the changed core files, and searched the tree for remaining Comments/CommentForm imports from emdash/ui. I found one real gap: the canonical skills/building-emdash-site/ docs were updated, but the copies embedded in every template under templates/*/.agents/skills/building-emdash-site/ still instruct users to import comments from the old emdash/ui barrel. Since those copies are tracked files with no sync script, they are now stale and will break (or mislead) anyone scaffolding a site from a template.
I also noted that, while the integration/e2e fixtures now exercise emdash/ui/comments implicitly, there is no automated regression guard for the actual CSS-isolation claim. Given the difficulty of asserting over Astro's emitted CSS, this is a suggestion rather than a blocker.
Conclusion: the fix is sound, but the stale template skill copies need to be updated before merge.
Findings
-
[needs fixing]
templates/blank/.agents/skills/building-emdash-site/SKILL.md:102-111The canonical
skills/building-emdash-site/SKILL.mdwas updated to importComments/CommentFormfromemdash/ui/comments, but the embedded copy inside every template still shows the old barrel import.import { PortableText, Image, WidgetArea, EmDashHead, EmDashBodyStart, EmDashBodyEnd, } from "emdash/ui"; import { Comments, CommentForm } from "emdash/ui/comments"; import LiveSearch from "emdash/ui/search";The same stale example exists in
templates/blank/.agents/skills/building-emdash-site/references/site-features.md(line 291) and in the correspondingbuilding-emdash-sitecopies undertemplates/blog/,templates/blog-cloudflare/,templates/marketing/,templates/marketing-cloudflare/,templates/portfolio/,templates/portfolio-cloudflare/,templates/starter/, andtemplates/starter-cloudflare/. Those are real tracked files (not symlinks or generated artifacts), so they need the same update. -
[suggestion]
packages/core/src/components/index.ts:25-26The structural fix is clear from this comment, but there is no automated guard against someone re-adding
Comments/CommentFormto this barrel in the future. The PR's verification is a manual build comparison; consider adding a regression check (e.g., a build test that greps emitted CSS forec-commentselectors on a page that does not importemdash/ui/comments, or a static check thatcomponents/index.tsdoes not re-export the comment components).
khoinguyenpham04
left a comment
There was a problem hiding this comment.
Thanks for fixing this, the approach looks good. Just a few things before merging, please check my comments.
Happy to help with the fixes as well.
| "default": "./dist/astro/middleware/redirect.mjs" | ||
| }, | ||
| "./ui": "./src/ui.ts", | ||
| "./ui/comments": "./src/ui-comments.ts", |
There was a problem hiding this comment.
CI is failing because this points to a raw TypeScript file.
| ```astro | ||
| --- | ||
| import { Comments, CommentForm } from "emdash/ui"; | ||
| import { Comments, CommentForm } from "emdash/ui/comments"; |
There was a problem hiding this comment.
Looks like the template copies still have the old import. Try running ./scripts/sync-template-skills.sh should update them all.
|
My concern here is that this is a breaking change. Any site that uses comments will break if they upgrade. I think it doesn't need to be: we can add the subpath export for this and the other components, while leaving the barrel untouched. We can then update templates to import from e.g. |
|
I was okay with this being a pre-1.0 breaking change. Less maintenance burden moving forward, no? |
|
We can remove the barrel in 1.0, but I want to avoid breaking changes unless they're needed. This time I'd suggest addign subpath exports, changing the barrel re-exports to use |
Their <style> blocks were pulled into a shared, render-blocking CSS chunk on every page that imported anything from the emdash/ui barrel (e.g. PortableText), because Astro scans the whole barrel module graph. Splitting them into a dedicated entry point keeps comment CSS off pages that don't render comments, mirroring the existing emdash/ui/search entry point.
Address ascorbic review: add emdash/ui/comments without breaking existing emdash/ui imports. Mark barrel re-exports @deprecated (remove in 1.0), allowlist the subpath in the public-source guard, and sync template skills.
375bce7 to
201e4b2
Compare
|
Updated per @ascorbic's guidance:
Also rebased onto latest |
What does this PR do?
Adds a dedicated
emdash/ui/commentsentry point forCommentsandCommentForm, so their CSS no longer bleeds onto pages that don't render comments — while keeping the barrel re-exports for backwards compatibility.Astro's CSS scanner processes the entire barrel module when any export is imported. Because
Comments.astroandCommentForm.astrowere only available viaemdash/ui's barrel, a page that only didimport { PortableText } from "emdash/ui"pulled their<style>blocks into a shared, render-blocking CSS chunk on every page. This mirrors the existingemdash/ui/searchentry point.Non-breaking (per @ascorbic): the barrel still re-exports
Comments/CommentFormwith@deprecatedJSDoc pointing atemdash/ui/comments. Templates, demos, fixtures, and docs use the new subpath. Barrel re-exports drop in 1.0.Closes #2039
Reproduction / verification
Built
demos/simple(astro build, SSR) before and after the change and grepped the emitted client CSS forec-commentselectors (when demos import from the subpath):Base.*.css(shared layout — every page)index.*.css(home)styles.*.css(global)_slug_.*.css(posts page that renders comments)Migration (recommended)
Existing
emdash/uiimports continue to work until 1.0.Type of change
Checklist
pnpm typecheckpasses (packages, demos, templates)pnpm lintpassespnpm testpassespnpm formathas been runAI-generated code disclosure
Screenshots / test output
See the before/after CSS table above. Follow-ups from review:
@deprecated(non-breaking)./ui/commentstoRUNTIME_COUPLED(fixes typecheck public-source guard)./scripts/sync-template-skills.shfor embedded skill copiesTry this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
fix/2039-comments-ui-subpath. Updated automatically when the playground redeploys.